Skip to content

Remove caBundle placeholder in CRD webhooks #332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove caBundle placeholder in CRD webhooks #332

wants to merge 1 commit into from

Conversation

@sharath-srikanth-chellappa
Copy link

@sharath-srikanth-chellappa sharath-srikanth-chellappa commented Sep 19, 2025

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #331

Special notes for your reviewer:

Changes Made:

  • Removes the caBundle: Cg== placeholder from config/crd/patches/webhook_in_kubevirtclusters.yaml
  • Removes associated comment explaining the placeholder's purpose
  • Removes the caBundle: Cg== placeholder from config/crd/patches/webhook_in_kubevirtmachines.yaml
  • Removes associated comment explaining the placeholder's purpose

Starting with Kubernetes v1.31, the API server no longer accepts the newline placeholder (Cg==) as a valid caBundle value, causing webhook registration failures.

As documented in Kubernetes issue #125569, the Kubernetes API server behavior changed to be more strict about caBundle validation.

Release notes:

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 19, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: sharath-srikanth-chellappa / name: Sharath Srikanth Chellappa (6117185)

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 19, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sharath-srikanth-chellappa
Once this PR has been reviewed and has the lgtm label, please assign justinsb for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Sep 19, 2025
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 19, 2025
@sharath-srikanth-chellappa sharath-srikanth-chellappa force-pushed the user/sharathsr/remove-cabundle-placeholder branch from 9821ed5 to 6117185 Compare September 22, 2025 20:31
@nunnatsa nunnatsa mentioned this pull request Sep 25, 2025
Open
@sharath-srikanth-chellappa
Copy link
Author

sharath-srikanth-chellappa commented Sep 29, 2025

Apologies for closing it. I needed to reach out for some more details before opening this PR. I have the clarity and this can now be reopened.

@nunnatsa

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 29, 2025
@nunnatsa
Copy link
Contributor

nunnatsa commented Sep 29, 2025

thanks @sharath-srikanth-chellappa

After looking at it a bit deeper, I'm not sure it's needed. I need to check why these files even exist, when we only have one API version, and we don't need any conversion webhook, that is not implemented anyway.

I suspect it's kind of auto generated.

@sharath-srikanth-chellappa
Copy link
Author

sharath-srikanth-chellappa commented Sep 29, 2025

Hi @nunnatsa

I agree with the fact that we would not need this autogenerated webhook given that we have a single API version.

However we are calling a function to setup webhooks in main.go (webhookhandler.SetupWebhookWithManager) for the KubevirtMachine -
https://github.com/kubernetes-sigs/cluster-api-provider-kubevirt/blob/main/main.go#L168

Thats the reason I had raised the PR to unblock us for the time being while we moved towards removing the support for the webhooks later on

@nunnatsa
Copy link
Contributor

nunnatsa commented Sep 29, 2025

This function creates a validation webhook, not a conversion webhook. So it's not relevant to these files

@sharath-srikanth-chellappa
Copy link
Author

sharath-srikanth-chellappa commented Sep 29, 2025 •
edited
Loading

Aren't the conversion webhooks generated by kubebuilder by the following marker kubebuilder:storageversion - https://book.kubebuilder.io/multiversion-tutorial/api-changes.html?highlight=kubebuilder%3Astorageversion#storage-versions

I see this listed as a part of api/v1alpha1/kubevirtcluster_types.go ,api/v1alpha1/kubevirtmachine_types.go and api/v1alpha1/kubevirtmachinetemplate_types.go

Please correct me if I am wrong here.

@sharath-srikanth-chellappa
Copy link
Author

sharath-srikanth-chellappa commented Oct 13, 2025

@nunnatsa Do you have any thoughts here. I could put up a new PR to remove the files altogether if that's how you want to do it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nunnatsa nunnatsa Awaiting requested review from nunnatsa

@qinqon qinqon Awaiting requested review from qinqon

Assignees

No one assigned

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

caBundle placeholder no longer supported

3 participants

@sharath-srikanth-chellappa @k8s-ci-robot @nunnatsa